-
-
Notifications
You must be signed in to change notification settings - Fork 92
Change insertion delimiter for javascript call scope #2759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
phillco
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tldr: easy fix, was always wrong, but Andreas (and presumably pokey) would just use "clone state" instead of "clone call" so they didn't run into this
|
@pokey I was good on merging this, but then after discussing the underlying issue with Andreas, sort of realized it's a bit more ambiguous then I realized. The problem is that a call could be inside of the list, or a statement on its own, and there's not a single insertion delimiter that covers both (list would be Then we considered that the user should really just be targeting the specific scope that they want in the first place (item, statement, etc), rather than clone call assuming that they want a statement and inserting On the other hand, the majority of the time, probably duplicating a statement is what they are trying to do and maybe this is a probabilistic quality life improvement. What's the precedence for this sort of design question? |
|
yeah in my opinion there is no natural delimiter for call, as it is not naturally a part of any delimited sequence. As you point out, a call could be an argument or could be the rhs of a statement just as easily as it could be a complete statement. But there is always the counterargument that it is better to do something useful than nothing at all, even if it is not well-founded from a theoretical perspective. My inclination would be not to have an insertion delimiter, but I don't feel strongly |
|
sgtm |
|
fwiw, if the call was in an argument list or the right hand side of an assignment, 'clone call' wouldn't make much sense to do-in the former I think one would clone the argument (rather than the call), and in the latter I think you would clone the statement most likely (or perhaps the value)-I can hardly think of a situation where you would want to clone the call and add a space between them. I think there is a difference between clone and other operations here where cloning a call is a special case and the behavior may make sense for it to be different because it is a clone operation rather than something else. The most sensible place to insert a call would be another statement, though I recognize that it is an ambiguous case. |
Fixes #2650
Checklist